Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix label treatment of related charms #277

Merged
merged 4 commits into from
Feb 26, 2024
Merged

Conversation

sed-i
Copy link
Contributor

@sed-i sed-i commented Feb 23, 2024

Issue

This charm overwrites existing topology labels with its own topology labels. This happens because:

  1. This charm doesn't have cos-tool, so injection of label matchers is skipped.
  2. This charm has an outdated labeling logic: here, all alert labels are being replaced by this charm's labels, whereas in cos-lib only missing labels are added.
  3. This charm has two different use cases for rules that are currently not captured well:
  4. Like most charms, this charm has built-in rules and dashboards that need to be enriched with the charm's topology labels.
  5. Like cos-config, this charm needs to forward 3rd-party rules and dashboards without changing the labels.

Solution

  • Add cos-tool to charm.
  • Fetch lib loki_push_api.v1

Fixes #275.

Context

This PR goes in tandem with:

Testing Instructions

See canonical/loki-k8s-operator#353.

Revision 61 of 'grafana-agent-k8s' released to edge/pr-277 (attaching resources: 'agent-image' r38)

Upgrade Notes

No change further action beyond juju refresh is needed – labels should be corrected after an upgrade.

@sed-i sed-i force-pushed the feature/fix-label-overwrite branch from ec4cf6f to e4d507f Compare February 23, 2024 14:22
@sed-i sed-i marked this pull request as ready for review February 23, 2024 19:05
@sed-i
Copy link
Contributor Author

sed-i commented Feb 24, 2024

@rgildein if you're able to test this change,

Revision 61 of 'grafana-agent-k8s' released to edge/pr-277 (attaching resources: 'agent-image' r38)

@dstathis dstathis merged commit 0427bda into main Feb 26, 2024
13 checks passed
@dstathis dstathis deleted the feature/fix-label-overwrite branch February 26, 2024 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong juju_topology in alert rule provided via LogProxyConsumer
3 participants